Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: add basic mqtt support #2412

Open
wants to merge 10 commits into
base: future3/develop
Choose a base branch
from

Conversation

c0un7-z3r0
Copy link
Contributor

@c0un7-z3r0 c0un7-z3r0 commented Aug 3, 2024

This adds initial support and functionality for MQTT. Trying to be downward compatible as much as possible so potential mqtt implementations are not totally broken. Not all legacy features could be ported since the current future3 is not supporting them. Yet.

This PR referes to #1963

@c0un7-z3r0 c0un7-z3r0 changed the title feat: add basic mqtt support feat: add basic mqtt support #1963 Aug 3, 2024
@coveralls
Copy link

coveralls commented Aug 3, 2024

Pull Request Test Coverage Report for Build 12244417857

Warning: This coverage report may be inaccurate.

This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.

Details

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 38.14%

Totals Coverage Status
Change from base Build 9427235714: 0.0%
Covered Lines: 492
Relevant Lines: 1290

💛 - Coveralls

@c0un7-z3r0 c0un7-z3r0 changed the title feat: add basic mqtt support #1963 feat: add basic mqtt support Aug 3, 2024
@s-martin s-martin added enhancement future3 Relates to future3 development labels Aug 3, 2024
Copy link
Collaborator

@s-martin s-martin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks for your contribution!

Please add a short doc file in documentation/builders/components, which helps users how to use the new MQTT feature.

src/jukebox/components/mqtt/mqtt_command_alias.py Outdated Show resolved Hide resolved
Copy link
Collaborator

@pabera pabera left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, it took me a while to get back to Phoniebox coding. This is a great addition. I have made only a few comments, the quality of code looks alright for now.

How do you use this code? What does your integration look like? Am I correct that this code does not work with Home Automations out of the box yet?

Comment on lines 205 to 209
client_id = cfg.setndefault("mqtt", "client_id", value="phoniebox-future3")
username = cfg.setndefault("mqtt", "username", value="phoniebox-dev")
password = cfg.setndefault("mqtt", "password", value="phoniebox-dev")
host = cfg.setndefault("mqtt", "host", value="127.0.0.1")
port = cfg.setndefault("mqtt", "port", value=1883)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we have some of those useful settings in jukebox.yaml?
Also, one addition I would prefer is to be able to enable/disable this functionality (e.g. through the jukebox.yaml)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good point. I added an enable flag in the jukebox.yaml config of this component.

example config

modules:
    named:
        ...
        mqtt: mqtt
...
mqtt:
    enable: true
    # The client id used in communication with the MQTT broker and identification of the phoniebox
    client_id: phoniebox_dev
    # The username to authenticate against the broker
    username: phoniebox-dev
    # The password to authenticate against the broker
    password: secret-password
    # The host name or IP address of your mqtt broker
    host: 127.0.0.1
    # The port number of the mqtt broker. The default is 1883
    port: 1883

@c0un7-z3r0
Copy link
Contributor Author

@s-martin I added a short documentation about the MQTT feature.

@pabera The implementation is working and can be used over MQTT already. Home Assistant is not natively supporting MQTT Media Players so I build in the past a little HACS add on to allow the control of phoniebox via mqtt. https://github.com/c0un7-z3r0/hass-phoniebox

This extension is more or less a mapping of the MQTT events to the internal home assistant media player implementation. But the same can be achieved with the Universal Media Player from HA just needs manual mapping of events.

Not sure if other Home Automation Systems support MQTT Media Players out of the box.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement future3 Relates to future3 development
Projects
None yet
Development

Successfully merging this pull request may close these issues.

🚀 | Add MQTT component to support control via Smart Home solution
4 participants